London | 26-ITP-Sep | Shirin Panahian | Sprint 1 | Complete sprint 1 exercises by shirinpanahian · Pull Request #1498 · CodeYourFuture/Module-Data-Groups

shirinpanahian · 2026-11-06T11:41:04Z

Learners, PR Template

Self checklist

I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
My changes meet the requirements of the task
I have tested my changes
My changes follow the style guide

Task code

CYF-1071

Changelist

Implemented dedupe: Removes duplicate elements from an array while preserving the original order and first occurrence of each element. Handles empty arrays and arrays without duplicates.
Implemented findMax: Finds and returns the largest number in an array, with handling for empty arrays, single-element arrays and invalid inputs.
Implemented sum and mean: Added the functions to calculate the sum and arithmetic mean of an array of numbers.
Implemented calculateMedian: Calculates the median of an array of numbers, supporting both odd and even numbers of elements. Includes input validation and error handling.
Implemented describeMedian: Returns a descriptive message containing the calculated median, or an appropriate error message when the median cannot be calculated.
Refactored includes: Updated the implementation to use a for...of loop.
Added and updated tests: Expanded test coverage for normal cases, empty arrays, single-element arrays, duplicate elements and invalid inputs.

cjyuan

Function implementations are all correct. Well done.
Regarding the PR description, could you use AI to research what should be included in the "Changelist" section of a PR, then revise your Changelist content accordingly?

cjyuan · 2026-11-06T17:59:30Z

+  }
+
+  for (const item of list) {
+    if (typeof item !== "number" || !Number.isFinite(item)) {


Very thoughtful to take Infinity and NaN into consideration, but one of these checks is redundant.

Thanks for the feedback! You're right. I've removed the redundant check and kept Number.isFinite().

cjyuan · 2026-11-06T18:02:07Z

+  const copy = [...list];
+  const middleIndex = Math.floor(listLength / 2);
+  copy.sort((a, b) => a - b);


Could also use Array.prototype.toSorted().

cjyuan · 2026-11-06T18:04:28Z

+    if (newList.includes(item)) {
+      continue;
+    } else {
+      newList.push(item);
+    }


Note: Code on lines 7-11 could also be expressed without using else.

Thanks for the suggestion! I've simplified the code by removing the unnecessary else.

cjyuan · 2026-11-06T18:15:28Z

+  // Given an array with no duplicates
+  // When passed to the dedupe function
+  // Then it should return a copy of the original array
+  it("Given an array with no duplicates, it returns a copy of the original array", () => {
+    expect(dedupe(["d", "s", "t", "c"])).toEqual(["d", "s", "t", "c"]);
+    expect(dedupe([5, 4, 3, 2, 1])).toEqual([5, 4, 3, 2, 1]);
+    expect(dedupe([7, 8, 9])).toEqual([7, 8, 9]);
+  });


Your function implementation is correct. However, this test could be improved to better ensure
that any future changes continue to align with the expected behavior described on line 32:

Then it should return a copy of the original array

This test should fail if the function returns the original array (instead of a copy of the original array).

The current tests check only if both the original array and the returned array contain identical elements.
In order to validate the returned array is a different array, we need an additional check.

Could you find out what this additional check is, and then implement it for one of the test cases?

Thanks for the suggestion! I've added an additional check using not.toBe() to ensure the function returns a new array rather than the original one.

cjyuan · 2026-11-06T18:18:37Z

-test.todo("given an empty array, returns -Infinity");
+describe("findMax", () => {
+  it("given an empty array,returns -Infinity", () => {
+    expect(findMax([])).toEqual([-Infinity]);
+  });


The expected return value is a value of type "number" and not an array.

Thanks! I've made the suggested changes.

cjyuan · 2026-11-06T18:22:02Z

+  for (const item of elements) {
+    if (maxNum < item) {
+      maxNum = item;
+    }
+  }


Note: This for loop alone could handle an array of any size if you initialise maxNum to the smallest possible value.

Thanks for the suggestion! I've updated the initial value to -Infinity.

cjyuan · 2026-11-06T18:23:25Z

How should the function handle an array containing NaN?

Arrays containing NaN should throw an error. I've handled this using Number.isFinite().

cjyuan · 2026-11-06T18:24:23Z

What should calculateMean([Infinity, 0, -Infinity]) or calculateMean([0, NaN]) return?
The same also applies to sum().

Thanks! I've updated the validation to reject non-finite numbers and added tests.

cjyuan · 2026-11-06T18:25:58Z

+    sum = sum + item;
+  });
+  mean = sum / list.length;
+  return Number(mean.toFixed(2));


Why reduce the precision of the result?

The tests expect results rounded to 2 decimal places, so I've kept the rounding to match the expected behaviour.

cjyuan · 2026-11-06T18:26:29Z

+  it("given an array with negative or decimal numbers, returns the correct mean", () => {
+    expect(calculateMean([-2, -6, -76])).toEqual(-28);
+    expect(calculateMean([10, 0, -11])).toEqual(-0.33);
+    expect(calculateMean([6.87, 3.12, -11.25])).toEqual(-0.42);
+  });


Decimal numbers in most programming languages (including JS) are internally represented in "floating point number" format. Floating point arithmetic is not exact. For example, the result of 46.5678 - 46 === 0.5678 is false because 46.5678 - 46 only yield a value that is very close to 0.5678. Even changing the order in which the program add/subtract numbers can yield different values.

So the following could happen

expect( 1.2 + 0.6 + 0.005 ).toEqual( 1.805 ); // This fail expect( 1.2 + 0.6 + 0.005 ).toEqual( 1.8049999999999997 ); // This pass expect( 0.005 + 0.6 + 1.2 ).toEqual( 1.8049999999999997 ); // This fail console.log(1.2 + 0.6 + 0.005 == 1.805); // false console.log(1.2 + 0.6 + 0.005 == 0.005 + 0.6 + 1.2); // false

Can you find a more appropriate way to test a value (that involves decimal number calculations) for equality?

Suggestion: Look up

Checking equality in floating point arithmetic in JavaScript

Checking equality in floating point arithmetic with Jest

Thanks for the explanation! I've updated the decimal number tests to use Jest's toBeCloseTo().

shirinpanahian added 11 commits November 5, 2026 11:00

Fix calculateMedian function and pass tests

91cb53c

Handle empty array in dedupe function

84b3b38

code work for no duplicate array

ee07d79

"Handle arrays without duplicates"

0bbf183

complete dedupe function

81b6d82

Test findMax with empty and single-element arrays

0aeba8f

complete function and test

0f21d3d

mean function and acceptance criteria

133a259

code work for throw error

0a9d415

Add median description and error handling tests

1842096

Refactor includes to use for...of loop

8210bdd

This comment has been minimized.

Sign in to view

shirinpanahian added 📅 Sprint 1 Assigned during Sprint 1 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Nov 6, 2026

This comment has been minimized.

Sign in to view

github-actions Bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Nov 6, 2026

This comment has been minimized.

Sign in to view

delete file

bae2058

shirinpanahian added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Nov 6, 2026

cjyuan added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Nov 6, 2026

cjyuan reviewed Nov 6, 2026

View reviewed changes

cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Nov 6, 2026

shirinpanahian added 2 commits November 6, 2026 23:45

Improve number validation and floating-point tests

68fe295

Improve code

92d12fe

shirinpanahian added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Nov 6, 2026

Conversation

shirinpanahian commented Nov 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Learners, PR Template

Task code

Changelist

Changelist

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

shirinpanahian commented Nov 6, 2026 •

edited

Loading